Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update DriverOptions.java #1256

Merged
merged 1 commit into from
Aug 17, 2020
Merged

Update DriverOptions.java #1256

merged 1 commit into from
Aug 17, 2020

Conversation

luizvaz
Copy link
Contributor

@luizvaz luizvaz commented Aug 16, 2020

🚀 Custom chrome options added.
🐞 Fixed a issue with workingDir that mistakenly create new directories when userDataDir was defined.

Description

Thanks for contributing this Pull Request. Make sure that you submit this Pull Request against the develop branch of this repository, add a brief description, and tag the relevant issue(s) and PR(s) below.

  • Relevant Issues : (compulsory)
  • Relevant PRs : (optional)
  • Type of change :
    • New feature
    • Bug fix for existing feature
    • Code quality improvement
    • Addition or Improvement of tests
    • Addition or Improvement of documentation

🚀 Custom chrome options added.
🐞 Fixed a issue with workingDir that mistakenly create new directories when userDataDir was defined.
userDataDir = (String) options.get("userDataDir");
disableNotifications = get("disableNotifications", false);
userAgent = get("userAgent", null);
String place = get("userDataDir", null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted user to provide null in which case chrome would use system defaults
and not providing would default to workingDir

will this work the same way

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving workingDir like that, will create countless 'chrome_1234567890' folders inside target with zero content.

Beacause Command class will check if it exists and create one if not.

This way, workingDir and userDataDir are the same.

@ptrthomas ptrthomas merged commit f2222b3 into karatelabs:develop Aug 17, 2020
Comment on lines +84 to +85
public final boolean disableNotifications;
public final String userAgent;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can ignore that!

Comment on lines +172 to +173
disableNotifications = get("disableNotifications", false);
userAgent = get("userAgent", null);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this too!

ptrthomas added a commit that referenced this pull request Aug 17, 2020
@ptrthomas
Copy link
Member

@luizvaz can you review this: a570297

when user passes userDataDir: null it will use the system install of chrome

also I removed the check for . and absolutePath, it should work without it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants